Skip to content
This repository has been archived by the owner on Oct 11, 2022. It is now read-only.

Add error handling middleware #43

Merged
merged 1 commit into from
Nov 19, 2015
Merged

Add error handling middleware #43

merged 1 commit into from
Nov 19, 2015

Conversation

kahmali
Copy link
Collaborator

@kahmali kahmali commented May 31, 2015

  • Add rest-json-error-handler package for handling Connect errors

See the discussion here for more context on this PR: #40 (comment)

* @middleware
*/
RestMiddleware.handleErrorAsJson = function (error, request, response, next) {
console.log('************ handleErrorAsJson ***************');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'll remove this unnecessary log statement. Just had it there for some cheap debugging.

@kahmali kahmali mentioned this pull request May 31, 2015
@kahmali kahmali changed the title Add error handling middleware [WIP] Add error handling middleware Jun 1, 2015
kahmali referenced this pull request Jun 2, 2015
This is a follow-up of PR #32.

General:
- Refactor tests:
  - Namespace all tests into their separate packages (for better
    organization of the output), including a subspace for middleware
  - Rename `expect` function to `waitFor` in all calls to
    testAsyncMulti(), to better reflect its behavior
- Copy .jshintrc into new packages

json-routes:
- Export RestMiddleware object for middleware packages to have a
  namespace to declare themselves within
- Add a `JsonRoutes.middleware` to eventually replace
  `JsonRoutes.middleWare` (since 'middleware' is one word)
- Update change log
- Remove 100 character line limit in README

rest-accounts-bearer-token:
- Split rest-accounts-bearer-token middleware package into a middleware
  package that parses a standard bearer token (rest-bearer-token-parser)
  and one that validates an auth token and sets the request.userId to
  the authorized Meteor.user's ID (authenticate-meteor-user-by-token)
- Attach middleware to RestMiddleware namespace, instead of adding it to
  every route. This allows middleware to be made available by simply
  adding its package. It can later be added to the middleware stack with
  JsonRoutes.middleware.use('path', RestMiddleware.<middlewareFunction>).
- Move tests to their appropriate package

simple-rest:
- Add token and auth middleware in tests to pass auth tests
  - This could be a sign that auth tests may be better suited in the
    corresponding middleware packages

rest-accounts-password:
- Use the latest version of json-routes (1.0.3)
- Add token parsing and auth middleware to the stack (currently added to
  all routes, which is bad - needs fix)
@stubailo stubailo mentioned this pull request Jun 2, 2015
7 tasks
@aldeed
Copy link
Collaborator

aldeed commented Sep 17, 2015

Hey @kahmali I'm trying to reboot getting this stuff merged and 3.0 released. Do you want to rebase and finish cleanup on this or should I take it over?

@kahmali kahmali force-pushed the error-middleware branch 2 times, most recently from e741877 to 0f16b4c Compare September 18, 2015 23:46
@kahmali
Copy link
Collaborator Author

kahmali commented Sep 18, 2015

Hey Eric!

Hope all is well. Glad to see you reviving this. I'm swamped right now, as I'm a few weeks out from an MVP launch, and I'm just trying to get all my ducks in a row with that. So, unfortunately, I won't be able to contribute much, if at all, on this for the next few weeks.

I did go ahead and rebase onto the latest and clean up the few things I mentioned above. I got it situated with jshint, except for the one error I mentioned in the commit message. The other known issue mentioned in the commit still exists, and the error handling middleware doesn't actually work until that gets resolved, so this isn't ready to be merged in yet. I'm sure you'll be able to take it from here and get it working.

Sorry I can't be more help with this at the moment. Restivus is now at a point where it can share middleware with simple:rest, so as soon as I have some time I hope to do my part to push both projects along.

@aldeed aldeed force-pushed the error-middleware branch 3 times, most recently from d2b3350 to 9616ea9 Compare November 19, 2015 02:08
- Use connectHandlers instead of rawConnectHandlers
- Add JsonRoutes.ErrorMiddleware.use for adding error handler middleware in the correct place
- Remove JsonRoutes.sendError and call next(error) instead so that middleware can handle it
- Export RestMiddleware as a place where packages can put their middleware functions by convention
- Update connect dependency to 2.30.2
- Create rest-json-error-handler package that defines RestMiddleware.handleErrorAsJson
@aldeed aldeed changed the title [WIP] Add error handling middleware Add error handling middleware Nov 19, 2015
aldeed added a commit that referenced this pull request Nov 19, 2015
@aldeed aldeed merged commit 2987b5e into devel Nov 19, 2015
@aldeed aldeed deleted the error-middleware branch November 19, 2015 05:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants